Skip to content

DRAFT: feat(accounts): recognize burn address in SNS token transfer#7831

Draft
yhabib wants to merge 8 commits intomainfrom
feat/burning-sns-tokens
Draft

DRAFT: feat(accounts): recognize burn address in SNS token transfer#7831
yhabib wants to merge 8 commits intomainfrom
feat/burning-sns-tokens

Conversation

@yhabib
Copy link
Copy Markdown
Contributor

@yhabib yhabib commented Apr 20, 2026

Motivation

SNS tokens can be burned by sending them to the minting account (burn address). When a user does this, no transaction fee is charged. The UI should recognize this scenario and reflect it clearly instead of displaying a regular fee.

Changes

  • Added the queryIcrcMintingAccount API function to fetch the minting account for an ICRC ledger canister.
  • Retrieved the minting account on modal mount in IcrcTokenTransactionModal and encoded it as a burn address string.
  • Introduced the burnAddress prop in TransactionModal and TransactionForm to identify when the destination matches the burn address.
  • Displayed the transaction fee as 0 with a "Burn address" label when the destination is the burn address.
  • Passed fee: 0n to the transfer call when sending to the burn address.
  • Added tests to cover burn label visibility, fee display, and correct fee in the transfer call.

Tests

  • Added unit tests

Todos

  • Accessibility (a11y) – any impact?
  • Changelog – is it needed?

@yhabib yhabib force-pushed the feat/burning-sns-tokens branch from ae017a0 to cd9b297 Compare April 24, 2026 07:36
@yhabib yhabib force-pushed the feat/burning-sns-tokens branch from cd9b297 to fd01f02 Compare April 24, 2026 11:59
@yhabib yhabib changed the title DRAFT: feat(transactions): detect burn address in ICRC token send flow DRAFT: feat(accounts): recognize burn address in SNS token transfer Apr 24, 2026
@yhabib yhabib requested a review from Copilot April 24, 2026 12:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds burn-address (minting account) recognition for SNS/ICRC token transfers so the UI can treat transfers to the minting account as burns (fee-free) and submit the transfer with fee: 0n.

Changes:

  • Added queryIcrcMintingAccount to fetch an ICRC ledger’s minting account and wired it into IcrcTokenTransactionModal on mount.
  • Introduced a burnAddress prop through TransactionModalTransactionForm to detect burn destinations and adjust UI/validation and transfer fee.
  • Added unit tests/page-object helpers covering burn label visibility and fee/transfer behavior.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/src/lib/api/icrc-ledger.api.ts Adds queryIcrcMintingAccount API wrapper to retrieve minting account.
frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte Fetches minting account on mount, derives burnAddress, and submits transfers with fee: 0n for burns.
frontend/src/lib/modals/transaction/TransactionModal.svelte Threads burnAddress into the form and suppresses fee display in the review step for burn destinations.
frontend/src/lib/components/transaction/TransactionForm.svelte Detects burn destination, uses effective fee 0 for max/validation, and shows a “Burn address” label while hiding the fee component.
frontend/src/lib/i18n/en.json Adds the new accounts.burn_address string.
frontend/src/lib/types/i18n.d.ts Extends i18n typing to include burn_address.
frontend/src/tests/lib/modals/accounts/IcrcTokenTransactionModal.spec.ts Adds burn-address test coverage (label visibility, fee handling, fee passed to transfer).
frontend/src/tests/page-objects/TransactionForm.page-object.ts Adds helpers to detect burn label and fee presence.
frontend/src/tests/lib/pages/SnsWallet.spec.ts Stubs queryIcrcMintingAccount to avoid real calls during wallet tests.
CHANGELOG-Nns-Dapp-unreleased.md Documents the burn-address UX change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/lib/components/transaction/TransactionForm.svelte Outdated
Comment thread frontend/src/lib/modals/transaction/TransactionModal.svelte
Comment thread frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte
Comment thread CHANGELOG-Nns-Dapp-unreleased.md
Comment thread frontend/src/tests/lib/modals/accounts/IcrcTokenTransactionModal.spec.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte Outdated
Comment thread frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte
Comment thread frontend/src/lib/components/transaction/TransactionForm.svelte
Comment thread frontend/src/lib/modals/transaction/TransactionModal.svelte
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte
Comment thread frontend/src/lib/modals/accounts/IcrcTokenTransactionModal.svelte Outdated
Comment thread frontend/src/lib/components/transaction/TransactionForm.svelte
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants